-
Notifications
You must be signed in to change notification settings - Fork 11.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[mlir][llvm] Do not inline variadic functions #77241
Conversation
This revision updates the llvm dialect inliner to explicitly disallow the inlining of variadic functions. Already previously the inlining failed if the number of function arguments did not match the number of call arguments. After the change, inlining checks the function is not variadic and it does not contain a va_start intrinsic.
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-llvm Author: Tobias Gysi (gysit) ChangesThis revision updates the llvm dialect inliner to explicitly disallow the inlining of variadic functions. Already previously the inlining failed if the number of function arguments did not match the number of call arguments. After the change, inlining checks the function is not variadic and it does not contain a va_start intrinsic. Full diff: https://github.com/llvm/llvm-project/pull/77241.diff 2 Files Affected:
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp
index 65c1daee6711ad..4a6154ea6d3004 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp
@@ -663,6 +663,10 @@ struct LLVMInlinerInterface : public DialectInlinerInterface {
<< "Cannot inline: callable is not an LLVM::LLVMFuncOp\n");
return false;
}
+ if (funcOp.isVarArg()) {
+ LLVM_DEBUG(llvm::dbgs() << "Cannot inline: callable is variadic\n");
+ return false;
+ }
// TODO: Generate aliasing metadata from noalias argument/result attributes.
if (auto attrs = funcOp.getArgAttrs()) {
for (DictionaryAttr attrDict : attrs->getAsRange<DictionaryAttr>()) {
@@ -704,7 +708,8 @@ struct LLVMInlinerInterface : public DialectInlinerInterface {
}
bool isLegalToInline(Operation *op, Region *, bool, IRMapping &) const final {
- return true;
+ // The inliner cannot handle variadic function arguments.
+ return !isa<LLVM::VaStartOp>(op);
}
/// Handle the given inlined return by replacing it with a branch. This
diff --git a/mlir/test/Dialect/LLVMIR/inlining.mlir b/mlir/test/Dialect/LLVMIR/inlining.mlir
index 63e7a46f1bdb06..3af8753bc318ab 100644
--- a/mlir/test/Dialect/LLVMIR/inlining.mlir
+++ b/mlir/test/Dialect/LLVMIR/inlining.mlir
@@ -644,3 +644,28 @@ llvm.func @caller(%ptr : !llvm.ptr) -> i32 {
llvm.store %c5, %ptr { access_groups = [#caller] } : i32, !llvm.ptr
llvm.return %0 : i32
}
+
+// -----
+
+llvm.func @vararg_func(...) {
+ llvm.return
+}
+
+llvm.func @vararg_intrinrics() {
+ %0 = llvm.mlir.constant(1 : i32) : i32
+ %list = llvm.alloca %0 x !llvm.struct<"struct.va_list_opaque", (ptr)> : (i32) -> !llvm.ptr
+ // The vararg intinriscs should normally be part of a variadic function.
+ // However, this test uses a non-variadic function to ensure the presence of
+ // the intrinsic alone suffices to prevent inlining.
+ llvm.intr.vastart %list : !llvm.ptr
+ llvm.return
+}
+
+// CHECK-LABEL: func @caller
+llvm.func @caller() {
+ // CHECK-NEXT: llvm.call @vararg_func()
+ llvm.call @vararg_func() vararg(!llvm.func<void (...)>) : () -> ()
+ // CHECK-NEXT: llvm.call @vararg_intrinrics()
+ llvm.call @vararg_intrinrics() : () -> ()
+ llvm.return
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Any idea how this could be supported eventually? I guess that this would only be possible by understanding and replacing the vararg intrinsics.
AFAIK neither GCC nor Clang support this... It would mean understanding the vararg intrinsics indeed. |
This revision updates the llvm dialect inliner to explicitly disallow the inlining of variadic functions. Already previously the inlining failed if the number of function arguments did not match the number of call arguments. After the change, inlining checks the function is not variadic and it does not contain a va_start intrinsic.
This revision updates the llvm dialect inliner to explicitly disallow the inlining of variadic functions. Already previously the inlining failed if the number of function arguments did not match the number of call arguments. After the change, inlining checks the function is not variadic and it does not contain a va_start intrinsic.